Repository navigation
Remove runtime layers, sidecars, port proxy, on-demand rootfs images - #109
Open
Jaana Dogan (rakyll) wants to merge 1 commit into
Open
Jaana Dogan (rakyll) wants to merge 1 commit into
Jaana Dogan (rakyll) wants to merge 1 commit into
Conversation
This was referenced Oct 19, 2026
Closed
Tomer Glottmann (tomergee)
left a comment
Tomer Glottmann (tomergee)
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: approve after one fix. The removal is complete and consistent. No dangling reference to a removed feature remains in code, CI, the Makefile, dependabot or the docs. The proto reserves field 4 and the name rootfs_image, and the generated Go and Python code agree with it. One item must change before merge: an old client that still sends rootfs_image gets a wrong environment and no error.
Before merge
internal/apiservice/server.go(CreateEnvironment) withproto/ateenv/v1alpha/env.proto:116-117: proto3 keeps a reserved field as unknown bytes and does not reject it. I marshaled aCreateEnvironmentRequestwith field 4 set, as an old client sends it.proto.Unmarshalreturns no error and keeps 38 unknown bytes.CreateEnvironmentdoes not look at them. Thus the server creates the environment from the base template, which runs the guest image, not the requested image. The caller gets no error, and its commands then fail for an unrelated reason. Old Python clients (Client.create(rootfs_image=...)) and older NeMo Gym providers send this field. Do these changes:- Reject a request that has unknown fields with
InvalidArgument, for exampleif len(req.ProtoReflect().GetUnknown()) > 0. - Name the replacement in the message: register an image template with
ate-env manifest template --rootfs-imageand pass its name. - Add a test that sends field 4 and expects
InvalidArgument.
- Reject a request that has unknown fields with
Nits
- PR description, "Security", sidecars: a sidecar comes from the template, and only a template author can set it. That author already controls the container image and its command, thus a sidecar adds no privilege for that author. The sidecar process also runs inside the CPU and memory limits of the actor. The exposure was the port proxy, which made the sidecar ports reachable. Consider stating it this way, because the reason for a later reintroduction depends on it.
- PR description: "There may be more changes to clean up". I found none.
go mod tidychanges nothing, staticcheck reports nothing, and the code that created ActorTemplates inate-env-apiis gone.GetActorTemplatestays, becauseserver.gostill uses it.
Verified
- Head f36dcf5 merges clean with
mainfccd920. GitHub CI is green: Go build, vet, test and license headers; Python client tests; Wiz and zizmor scans. - Local:
go build ./...,go vet ./...,go test ./...pass;go mod tidymakes no change;staticcheck ./...is clean. - Local Python (the
ci.yamlsteps): 116 passed, 26 skipped;ruff check clients/pythonis clean. git grepforrootfs_image,portproxy,sidecar,nemo-gym,swebench,benchmarking/,atenet-connect: the only hits are the keptate-env manifest template --rootfs-imagepath and its tests.- No Markdown link points to a removed path. The README keeps the
--rootfs-imagetemplate flow and tells operators to delete the<base>-<12 hex digits>templates that older versions derived. - Probe for item 1: a Go test that unmarshals a request with field 4 (removed after the run).
This removes a set of features that widened what ate-env-api and the guest can be made to do, without review in proportion to that reach. Our goal for this project is to keep the data plane APIs to a minimum and see if we can provide the guest services from Substrate actors only via config in the long-term. Removed: - Runtime layers and sidecars: `ate-env manifest template --layer`, `--sidecar` and `--sidecar-readyz`, and the guest's `-sidecar` and `-sidecar-readyz` support (cmd/ate-env-guest/sidecar.go). /readyz answers once the guest is listening. - Port proxy (internal/portproxy) and its ate-env-api flags `--atenet-connect`, `--port-proxy-trusted-origins` and `--port-proxy-response-header-timeout`, plus the CONNECT support in the fake router. - On-demand rootfs images: CreateEnvironmentRequest.rootfs_image (field 4 and its name are now reserved), the server-side derivation of `<base>-<digest>` ActorTemplates, `ate-env create --rootfs-image`, and `rootfs_image=` in the Python client. `ate-env manifest template --rootfs-image` stays: operators can still register image templates themselves. - The NeMo Gym integration (integrations/nemo-gym), which depended on the port proxy and on runtime layers. - SWE-bench benchmarking (cmd/benchmarking, internal/benchmarking, benchmarking/), which depended on on-demand rootfs images. - Docs for all of the above: docs/rootfs-images, examples/runtime-layers, and the related README sections. - CreateEnvironment now returns InvalidArgument when the request has unknown fields. The message names the replacement: register an image template with `ate-env manifest template --rootfs-image` and pass its name in template.name. Process: Changes of this size to a project like this, which adds new network surface, new privileges for the API service, new wire-protocol fields, and new integrations, need substantial design discussion and security review before they are merged. They should not land incrementally through feature PRs. Any of these capabilities can come back through a design proposal with a threat model and explicit maintainer sign-off. Compatibility: - Clients that set rootfs_image get an environment created from the base template, because the server drops the unknown field. Update them. - ate-env-api deployments that pass the removed port-proxy flags will fail to start; the generated manifest never set them. - Templates derived by earlier versions (`<base>-<12 hex>`) are unused and can be deleted with `kubectl-ate delete actor-template`.
Jaana Dogan (rakyll)
force-pushed
the
runtime-layers
branch
from
October 26, 2026 01:03
f36dcf5 to
6ee7e08
Compare
Jaana Dogan (rakyll)
changed the title
Collaborator
Author
|
Tomer Glottmann (@tomergee) Addressed the comments, PTAL. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This removes a set of features that widened what ate-env-api
and the guest can be made to do, without review in proportion
to that reach. Our goal for this project is to keep the data
plane APIs to a minimum and see if we can provide the guest
services from Substrate actors only via config in the long-term.
Removed:
ate-env manifest template --layer,--sidecarand--sidecar-readyz, and the guest's-sidecarand-sidecar-readyzsupport (cmd/ate-env-guest/sidecar.go). /readyzanswers once the guest is listening.
--atenet-connect,--port-proxy-trusted-originsand--port-proxy-response-header-timeout, plus the CONNECT support inthe fake router.
(field 4 and its name are now reserved), the server-side derivation
of
<base>-<digest>ActorTemplates,ate-env create --rootfs-image,and
rootfs_image=in the Python client.ate-env manifest template --rootfs-imagestays: operators can still register image templatesthemselves.
the port proxy and on runtime layers.
benchmarking/), which depended on on-demand rootfs images.
and the related README sections.
unknown fields. The message names the replacement: register an image
template with
ate-env manifest template --rootfs-imageand pass itsname in template.name.
Process:
Changes of this size to a project like this, which adds new network
surface, new privileges for the API service, new wire-protocol fields,
and new integrations, need substantial design discussion and security
review before they are merged. They should not land incrementally
through feature PRs. Any of these capabilities can come back through a
design proposal with a threat model and explicit maintainer sign-off.
Compatibility:
base template, because the server drops the unknown field. Update
them.
fail to start; the generated manifest never set them.
<base>-<12 hex>) are unusedand can be deleted with
kubectl-ate delete actor-template.